Repository navigation
Fix AST optionality - #65063
Fix AST optionality#65063Andrew Branch (andrewbranch) merged 3 commits into
Conversation
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the AST contract alignment and case/default clause separation.
No open issues 🎉
3 files not reviewed
tsc/internal/api/encoder/decoder_generated.go: Generated filetsc/internal/api/encoder/encoder_generated.go: Generated filetsc/internal/ast/ast_generated.go: Generated file
What changed in this PR
Addresses #64864 by aligning JavaScript API AST types with runtime values and updating the native compiler to use distinct case and default clause nodes.
Changes:
- Corrects AST property optionality and removes
expressionfromDefaultClause. - Ensures synthesized arrows have
=>tokens and initializes factory boolean values, modifier flags, and node-array transform flags. - Updates generators, serialization, compiler consumers, and regression tests to preserve these contracts.
| File | Description |
|---|---|
tsc/internal/transformers/moduletransforms/commonjsmodule.go |
Updates case and default clauses separately during transformation. |
tsc/internal/printer/printer.go |
Prints distinct clause types through shared statement accessors. |
tsc/internal/printer/printer_test.go |
Exercises arrow printing with an omitted token. |
tsc/internal/parser/parser.go |
Constructs separate case and default nodes. |
tsc/internal/ls/utilities.go |
Updates clause tracking to use the union alias. |
tsc/internal/ls/folding.go |
Uses shared clause statement accessors for folding. |
tsc/internal/ls/completions.go |
Updates clause types and completion factories. |
tsc/internal/checker/checker.go |
Reads fallthrough data from the shared clause base. |
tsc/internal/binder/binder.go |
Adapts clause binding and fallthrough storage. |
tsc/internal/ast/nodeaccessors_test.go |
Tests arrow tokens, clause accessors, traversal, and cloning. |
tsc/internal/ast/ast.go |
Updates expression and statement accessors for split clauses. |
tsc/internal/ast/ast_generated.go |
Regenerates clause structures, factories, dispatch, and optionality comments. |
tsc/internal/api/encoder/encoder_generated.go |
Encodes distinct child layouts for each clause type. |
tsc/internal/api/encoder/decoder_generated.go |
Decodes clauses using their separate factories. |
tools/scripts/tsc/generate-ts-ast.ts |
Generates factory defaults and computed modifier flags. |
tools/scripts/tsc/generate-go-ast.ts |
Generates arrow-token defaults and shared clause accessors. |
tools/scripts/tsc/ast.json |
Defines corrected optionality and separate clause schemas. |
packages/typescript/test/sync/ast.test.ts |
Adds runtime and type-level AST contract tests. |
packages/typescript/src/ast/visitor.generated.ts |
Removes expression visitation from default clauses. |
packages/typescript/src/ast/is.generated.ts |
Regenerates clause guards and adds a union guard. |
packages/typescript/src/ast/factory.generated.ts |
Applies corrected factory signatures, defaults, and clause handling. |
packages/typescript/src/ast/ast.ts |
Removes default clauses from HasExpression. |
packages/typescript/src/ast/ast.generated.ts |
Exposes corrected property types and clause interfaces. |
packages/typescript/src/api/node/protocol.generated.ts |
Aligns default-clause protocol metadata with its statement-only layout. |
Review effort: Balanced
This is the only item in the entire list that's a syntax error, so seems out of place? |
|
Good catch; I misremembered that as legal along with This does seem to be the only case where an optional node child property is always defined in error-free code, so there's no other precedent to go on. Other grammar errors like this are context-dependent, e.g. |
|
Tried a few different things, and we decided it would be worth looking into just making |
Jake Bailey (jakebailey)
left a comment
Jake Bailey (jakebailey)
left a comment
There was a problem hiding this comment.
Thanks. I realized a bug I was working on depended on this too
Fixes #64864
Changed
ExportAssignment.typeis now optional. Undefined forexport default 0;.PropertySignatureDeclaration.typeis now optional. Undefined forpininterface I { p; }.PropertySignatureDeclaration.initializeris now optional. Undefined forpininterface I { p: string; }.IndexSignatureDeclaration.typeis now optional. Undefined forinterface I { [key: string] /*: missing type here */; }.ArrowFunction.equalsGreaterThanTokenis now non-nil even for synthesized arrow functions, matching Strada's behavior.TaggedTemplateExpression.questionDotTokenis now optional. Undefined fortag`text`.PropertyAssignment.typeis now optional. Undefined forpinconst obj = { p: 1 };.ShorthandPropertyAssignment.typeis now optional. Undefined forpinconst obj = { p };.JSDocTemplateTag.constraintis now optional. Undefined for/** @template T */ function f() {}.JSDocSeeTag.nameExpressionis now optional. Undefined for/** @see */ function f() {}(missing reference).FunctionTypeNode.typeis now required. Intype F = () => number, it is thenumbertype node; parser recovery also supplies a node when the return type is missing.ConstructorTypeNode.typeis now required. Intype C = new () => object, it is theobjecttype node; parser recovery likewise supplies a node when missing.Separated
CaseClauseandDefaultClausein the schema and Go AST.CaseClause.expressionremains required;DefaultClausehas no expression property.CaseOrDefaultClauseis a union, with shared statements/flow data in a base.Factory-created boolean properties now default to
falserather thanundefined:Block.multiLine,ArrayLiteralExpression.multiLine,ObjectLiteralExpression.multiLine,ImportAttributes.multiLine,ExportDeclaration.isTypeOnly,SyntheticExpression.isSpread,JsxText.containsOnlyTriviaWhiteSpaces, andJSDocTypeLiteral.isArrayType.Factory-created node arrays now initialize
transformFlagsto0.Factory-created nodes now compute
modifierFlagsfrom their modifiers, matching parsed nodes:Nonewithout modifiers, otherwise the combined syntactic modifier bitmask.Not changed
node.parentis still typed as non-optional, which is a convenient lie inherited from Strada, and changing it would be pretty disruptive.